Skip to content

Conversation

ericproulx
Copy link
Contributor

@ericproulx ericproulx commented Oct 25, 2024

This PR replaces validator registration mechanism. Instead of relying on self.inherited, it registers validators by using const_get based on short name. Its does the same thing as the coercers.

Drawbacks

  • custom validator must be declared in Grape::Validations::Validators modules and class's name must finish by Validator.

In our README, the AlphaNumeric example would need to be in declared like this:

module Grape
  module Validations
    module Validators
      class AlphaNumericValidator < Base
        def validate_param!(attr_name, params)
          unless params[attr_name] =~ /\A[[:alnum:]]+\z/
            raise Grape::Exceptions::Validation.new params: [@scope.full_name(attr_name)], message: 'must consist of alpha-numeric characters'
          end
        end
      end
    end
  end
end

Benefits

  • Custom validators are declared the same way as the built-in ones.
  • Less confusion about where to add custom validators. Following the convention would naturally lead to something like lib/grape/validations/validators/custom_validator.rb
  • Registration done a when compiling instead of requiring

Replace validator inherited mechanism by dynamic const_get
Remove active_support/inflector
@grape-bot
Copy link

grape-bot commented Oct 25, 2024

1 Warning
⚠️ Unless you're refactoring existing code or improving documentation, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#2508](https://github.com/ruby-grape/grape/pull/2508): Update validator registration mechanism - [@ericproulx](https://github.com/ericproulx).

Generated by 🚫 Danger

@ericproulx ericproulx changed the title Dynamic validator registration Update validator registration mechanism Oct 25, 2024
@ericproulx ericproulx closed this Oct 27, 2024
@ericproulx ericproulx deleted the dynamic_validator_registration branch December 15, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants